Clean up header when tracing is disabled #7367
Conversation
| @@ -46,6 +46,10 @@ public static CallTargetState OnMethodBegin<TTarget, TRequest>(TTarget instance, | |||
| return new CallTargetState(scope); | |||
| } | |||
| } | |||
| else | |||
| { | |||
| headers.Remove(HttpHeaderNames.TracingEnabled); | |||
There was a problem hiding this comment.
This is nice in theory, but it could run into potential issues 🤔 e.g. there are often nested handlers, and if we remove this header from the first one, the innermost handler may create a span 🤔 I think instead we may need to implement the "black hole" span approach which has been discussed a few times, but which is non-trivial to implement at the moment 🤔
There was a problem hiding this comment.
Ok! Was worth a shot. I can close this out for now. But before I do...
Do you have any recommendations for folks in my position? I've tried delegating handlers as the PrimaryHttpMessageHandler, HttpClientHandler, but the headers are still present. I'd really like to have tracing enabled for my other services and the one I'm trying to instrument is a bit of a monolith, so there are a lot of other http requests coming in and out of it.
There was a problem hiding this comment.
Unfortunately, currently, the tracing (span generation) of HttpClient spans is tied to propagation. Assuming you do want to see spans for the requests, you just don't want to include the headers in the outgoing request, I think there's currently only one avenue:
- Disable tracing (and propagation) for the Url's you don't want to trace by setting
DD_TRACE_HTTP_CLIENT_EXCLUDED_URL_SUBSTRINGS, e.g.DD_TRACE_HTTP_CLIENT_EXCLUDED_URL_SUBSTRINGS=some.domain.com,some.other.domain/and/path. - Manually create a span around the calls to these
HttpClientrequests to emulate the span that you would have got.
The need to use the manual span is obviously a bit hacky, but I think it's the only real workaround available at the moment. Hope that helps, and we'll bear this request in mind as a possible new feature in the future, thanks!
There was a problem hiding this comment.
DD_TRACE_HTTP_CLIENT_EXCLUDED_URL_SUBSTRINGS is exactly what I needed! Totally worked. Didn't know that existed, for what it's worth, but I see it in the code now. Thanks a ton!
There was a problem hiding this comment.
Cool, glad that solved your issue 🙂
|
Doesn't fit the current paradigm: #7367 (comment) |
Summary of changes
When tracing is disabled, also remove the "tracing enabled" header.
Reason for change
We have a use-case where a partner doesn't allow unknown headers. This header allows the tracing headers to not be added, but it still remains when the request is sent, which breaks their system.
Implementation details
Test coverage
None that I found. This is more of a proof of concept to get the maintainers feedback. Happy to dig in more, if this is a good idea.
Other details